Skip to content

Conversation

@stenlarsson
Copy link
Contributor

@stenlarsson stenlarsson commented Dec 5, 2025

Rationale for this change

Many compute functions are missing their corresponding options class in the GLib bindings. Many functions can still be used in Ruby with reduced functionality, but some functions like extract_regex cannot be used at all.

What changes are included in this PR?

The following options classes are added to the GLib bindings:

  • GArrowAssumeTimezoneOptions
  • GArrowCumulativeOptions
  • GArrowDayOfWeekOptions
  • GArrowDictionaryEncodeOptions
  • GArrowElementWiseAggregateOptions
  • GArrowExtractRegexOptions
  • GArrowExtractRegexSpanOptions
  • GArrowJoinOptions
  • GArrowListFlattenOptions
  • GArrowListSliceOptions
  • GArrowMakeStructOptions
  • GArrowMapLookupOptions
  • GArrowModeOptions
  • GArrowNullOptions
  • GArrowPadOptions
  • GArrowPairwiseOptions
  • GArrowPartitionNthOptions
  • GArrowPivotWiderOptions
  • GArrowRankQuantileOptions
  • GArrowReplaceSliceOptions
  • GArrowReplaceSubstringOptions
  • GArrowRoundBinaryOptions
  • GArrowRoundTemporalOptions
  • GArrowSelectKOptions
  • GArrowSkewOptions
  • GArrowSliceOptions
  • GArrowSplitOptions
  • GArrowTDigestOptions
  • GArrowTrimOptions
  • GArrowWeekOptions
  • GArrowWinsorizeOptions

The following classes are added as well since they were needed in some tests:

  • GArrowFixedSizeListArray
  • GArrowFixedSizeListArrayBuilder

Are these changes tested?

All classes are tested with Ruby unit tests.

Are there any user-facing changes?

Yes, new classes.

@stenlarsson stenlarsson requested a review from kou as a code owner December 5, 2025 10:16
@github-actions github-actions bot added the awaiting review Awaiting review label Dec 5, 2025
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

⚠️ GitHub issue #48356 has been automatically assigned in GitHub to PR creator.

@stenlarsson
Copy link
Contributor Author

The MakeStructOptions class is missing the field_nullability and field_metadata properties because I ran into limitations of the gobject-introspection gem, e.g.: NotImplementedError: TODO: GIArgument(GList)[ghash] -> Ruby. I couldn't figure out what types the properties should have for it to work.

@stenlarsson stenlarsson force-pushed the even-more-glib-option-classes branch from 4620142 to 83251e0 Compare December 5, 2025 10:32
@stenlarsson
Copy link
Contributor Author

Another issue I ran into was that the ListSliceOptions property return_fixed_size_list has the type std::optional<bool>. While it would make sense in Ruby, I couldn't figure out how to set a boolean to null in GLib, so I created an enum instead.

@kou
Copy link
Member

kou commented Dec 5, 2025

Thanks! But could you split this large PR to smaller PRs? I can't review this...

Could you use one issue/PR per option class?

For example, I'm using #48132 is an umbrella issue for pure Ruby Apache Arrow implementation.

@kou
Copy link
Member

kou commented Dec 5, 2025

The MakeStructOptions class is missing the field_nullability and field_metadata properties because I ran into limitations of the gobject-introspection gem, e.g.: NotImplementedError: TODO: GIArgument(GList)[ghash] -> Ruby. I couldn't figure out what types the properties should have for it to work.

I'll implement it in the gobject-introspection gem later.

@stenlarsson
Copy link
Contributor Author

There is one commit per option class to make it easier to review. The reason I created a single PR is that 32 of the commits adds code to the same place. This means that when it is time to merge you would get conflicts for all of these. Also, two of them needs the 33rd commit (FixedSizeListArray) for the tests to pass. Do you still want me to create separate PRs?

@kou
Copy link
Member

kou commented Dec 5, 2025

Yes.

If you use one large PR, we can't merge this until we're ready all options. If any of them has a problem, we can't merge all of them. I can't review large PR multiple times.

@kou
Copy link
Member

kou commented Dec 5, 2025

You don't need to open 32 PRs at a time. You can open a few PR at once. You can open more PRs after they are merged.

@stenlarsson
Copy link
Contributor Author

That makes sense. I will close this and open separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants